Skip to content

[Rust] various enhancements#6411

Merged
wing328 merged 4 commits into
swagger-api:masterfrom
ahmedcharles:rust
Sep 1, 2017
Merged

[Rust] various enhancements#6411
wing328 merged 4 commits into
swagger-api:masterfrom
ahmedcharles:rust

Conversation

@ahmedcharles

Copy link
Copy Markdown
Contributor

No description provided.

@wing328

wing328 commented Aug 31, 2017

Copy link
Copy Markdown
Contributor

@ahmedcharles thanks for the PR.

cc @frol @farcaller

@farcaller

Copy link
Copy Markdown
Contributor

LGTM.

}

{{#vars}}
#[allow(non_snake_case)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wing328 Can we avoid this? I know that other Codegen languages transform names according to the guidelines of the target programming language.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I'm just new to swagger-codegen and wasn't sure how to get names exactly right.

In this case, I need two versions of the name, one with a leading underscore and one without, because the warning is due to having two underscores in a row. Do you know how to create two mustache variables, one with a leading underscore for reserved names and one without?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frol Is there any lib in Rust that can handle JSON (de)serialization?

@ahmedcharles please share the spec of the model.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wing328 Serde is used here for JSON (de)serialization.

@ahmedcharles You can just use _{{ var_name }} in the template if you need a leading underscore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frol I had a look at https://serde.rs/examples.html but couldn't an example how to provide a 1-1 mapping for attribute name (similar to what've done for Java, C#, etc). Do you know how we can map attribute name with Serde?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frol The issue with the warning is that type is a reserved word, so the {{name}} is _type, which when combined with with_{{name}} becomes with__type, which is what creates the warning. What I need is {{name}} = _type and {{rawName}} = type, so then I can do fn {{name}}(...) and fn with_{{rawName}}(...) and never have it generate names with two underscores in a row.

@wing328 Isn't that what #[serde(rename = "jsonName")] rust_name: String; does?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahmedcharles I think so. I missed that.

Client::configure().connector(HttpConnector::new(4, &handle)).build(&handle)));

let work = apicli.pet_api().add_pet(&new_pet)
let work = apicli.pet_api().AddPet(new_pet)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be not the change this PR introduced, but it would be great if the naming conventions are aligned according to the recommendations instead of putting #[allow(non_snake_case)] everywhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the python generator uses add_pet, though I didn't look at the Java code. Do you know how to change the Java code to get this to be the different style?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wing328 wing328 merged commit 60766c6 into swagger-api:master Sep 1, 2017
@wing328 wing328 changed the title Make the Rust codegen compile. [Rust] various enhancements Sep 1, 2017
wing328 added a commit that referenced this pull request Sep 1, 2017
wing328 added a commit that referenced this pull request Sep 1, 2017
…6421)

* Revert "Make the Rust codegen compile. (#6411)"

This reverts commit 60766c6.

* Revert "[Kotlin] Fix model enum generation (#6383)"

This reverts commit adf5d64.

* Revert "Global namespaces for ruby basic types (#6418)"

This reverts commit 070894d.

* Revert "add ehyche to swift tech comm"

This reverts commit 5c62ba1.

* Revert "[python-client] Thread pool fix (#6396)"

This reverts commit a28ce0b.

* Revert "update retrofit2 petstore samples"

This reverts commit 0f1a61d.

* Revert "Retrofit2: Return ResponseBody if response if file. (#6407)"

This reverts commit 481c040.
@frol

frol commented Sep 1, 2017

Copy link
Copy Markdown
Contributor

@ahmedcharles FYI, it seems that the PR was merged by mistake and it was reverted... Could you, please, continue your work on the better variable naming?

@frol

frol commented Sep 1, 2017

Copy link
Copy Markdown
Contributor

Hmm, well... I have just noticed that the naming conventions are fine! Great job!

@wing328 Why did you revert the PR?

@wing328

wing328 commented Sep 2, 2017

Copy link
Copy Markdown
Contributor

@frol it was not reverted: a6561e7

@ahmedcharles ahmedcharles deleted the rust branch September 3, 2017 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants